Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upgrade to Rails 6 #150

Merged
merged 2 commits into from
May 8, 2019
Merged

Upgrade to Rails 6 #150

merged 2 commits into from
May 8, 2019

Conversation

christiannelson
Copy link
Contributor

@christiannelson christiannelson commented Jan 25, 2019 •
edited
Loading

The first beta is out: https://edgeguides.rubyonrails.org/6_0_release_notes.html

We typically upgrade raygun-rails sometime during the beta period, once we have a sense of the stability of Rails.

Upgrading includes updating the rails gems and massaging any configuration to best match what would be in a new Rails 6 app. Thanks to @mattbrictson, I learned about railsdiff.org, which made finding other differences much easier. Maybe of which have been resolved in this PR.

Tasks

  • Upgrade Rails and related gems
  • Adopt new configuration options and structure
  • Explore defaulting to parallel testing (or make it very easy to do so): NOTE - Not supported by rspec, for either thread or process based parallelization.

Notes

  • Dropped sass-rails because it's deprecated, use sassc-rails only. rails g scaffold didn't raise an exception, so something has been fixed.
  • Did not choose to go all in on webpacker, even though it's the default in rails 6. I want to see what other people think about it and check it out myself first.

@mattbrictson
Copy link
Contributor

mattbrictson commented Jan 30, 2019 •
edited
Loading

I wonder if it would be convenient to fold the upgrade to bootstrap 4.2.1 (#137) into this PR? I think the bootstrap upgrade would benefit from some more thorough testing that would naturally be part of doing the Rails 6 migration.

@christiannelson
Copy link
Contributor Author

christiannelson commented Jan 30, 2019 via email

@christiannelson christiannelson mentioned this pull request Jan 30, 2019
Closed
@mattbrictson mattbrictson mentioned this pull request Feb 23, 2019
Merged
@christiannelson christiannelson force-pushed the rails-6 branch 3 times, most recently from 620ea63 to 35d49e8 Compare March 18, 2019 20:30
@christiannelson christiannelson force-pushed the rails-6 branch 12 times, most recently from d6e2a54 to c0f5619 Compare April 30, 2019 22:00
@mattbrictson
Copy link
Contributor

I think the setup/update scripts needs some updates because the FileUtils module is no longer included.

$ bin/setup 
== Installing dependencies ==
The Gemfile's dependencies are satisfied

== Copying sample files ==
Traceback (most recent call last):
	3: from bin/setup:11:in `<main>'
	2: from /Users/mbrictson/.rbenv/versions/2.6.3/lib/ruby/2.6.0/fileutils.rb:128:in `cd'
	1: from /Users/mbrictson/.rbenv/versions/2.6.3/lib/ruby/2.6.0/fileutils.rb:128:in `chdir'
bin/setup:22:in `block in <main>': undefined method `cp' for main:Object (NoMethodError)

@christiannelson
Copy link
Contributor Author

Did some research about using RSpec with the new built-in parallel testing in Rails 6. It's not good news, sadly: rspec/rspec-rails#2104

The issue is that rspec-core is not threadsafe and making it threadsafe is non trivial and not a priority.

Rails 6 supports both multi-process and multi-thread parallelization. So maybe there's a way to still support multi-process parallelization with RSpec and Rails 6? Will look into that a little more.

@christiannelson christiannelson self-assigned this May 2, 2019
seport
seport approved these changes May 2, 2019
View reviewed changes
Copy link
Contributor

@seport seport left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but maybe someone else should also take a 👀 at it too

@mattbrictson
Copy link
Contributor

I'm looking at the railsdiff – http://railsdiff.org/5.2.3/6.0.0.rc1 – and it seems that Rails 6 makes these changes to bin:

  • bin/bundle has been removed
  • bin/setup and bin/update have been consolidated into one idempotent bin/setup script

Should we mirror those changes here?

@christiannelson
Copy link
Contributor Author

@mattbrictson I didn't know about railsdiff. I will look for other differences between the versions too.

@mattbrictson
Copy link
Contributor

I think the other big difference is that Rails 6 drops JS from the asset pipeline in favor of webpacker. I am not sure we want to go that far. However if we continue with application.js etc. then we are diverging somewhat from "standard" Rails.

@christiannelson
Upgrade to Rails 6.0
ce23d2a 
Upgrade to Rails 6, including any relevant configuration changes.
@christiannelson
Some minor puma tweaks
6d5c972
mattbrictson-babylon
mattbrictson-babylon approved these changes May 8, 2019
View reviewed changes
Copy link

@mattbrictson-babylon mattbrictson-babylon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

mattbrictson
mattbrictson approved these changes May 8, 2019
View reviewed changes
Copy link
Contributor

@mattbrictson mattbrictson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve as well 😄

@christiannelson christiannelson merged commit 6d5c972 into master May 8, 2019
@christiannelson christiannelson deleted the rails-6 branch May 8, 2019 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattbrictson-babylon mattbrictson-babylon mattbrictson-babylon approved these changes

@seport seport seport approved these changes

@mattbrictson mattbrictson mattbrictson approved these changes

Assignees

@christiannelson christiannelson

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@christiannelson @mattbrictson @seport @mattbrictson-babylon